Skip to content

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Nov 27, 2024

Motivation:

The generated code doesn't consistently include documentation from the source IDL. It also doesn't explain the difference between each of the generated protocols and when to use them.

Modifications:

Add documentation to the generated code, and where possible, also include any documentation from the source IDL.

Result:

Better documented generated code

Motivation:

The generated code doesn't consistently include documentation from the
source IDL. It also doesn't explain the difference between each of the
generated protocols and when to use them.

Modifications:

Add documentation to the generated code, and where possible, also
include any documentation from the source IDL.

Result:

Better documented generated code
@glbrntt glbrntt added the 🔨 semver/patch No public API change. label Nov 27, 2024

private func explodedDocs(for method: MethodDescriptor) -> String {
let summary = "/// Call the \"\(method.name.base)\" method."
var parameters = """
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How often is this code called? Often enough to want to use something more efficient than +=?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Infrequently, it's only used at code generation time so not really concerned about this.

Copy link
Collaborator

@rnro rnro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good and like a really good usability improvement overall 🙂

@glbrntt glbrntt enabled auto-merge (squash) November 27, 2024 13:59
@glbrntt glbrntt merged commit 311486e into grpc:main Nov 27, 2024
43 of 45 checks passed
@rnro rnro mentioned this pull request Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants